Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing facts for Fedora 20, 21, and 26 #50

Merged
merged 1 commit into from
Sep 4, 2017
Merged

Add missing facts for Fedora 20, 21, and 26 #50

merged 1 commit into from
Sep 4, 2017

Conversation

blackknight36
Copy link
Contributor

Added fact files for Fedora 20, 21, and 26 to the facter 2.4 database.

@coveralls
Copy link

coveralls commented Sep 1, 2017

Coverage Status

Coverage remained the same at 73.529% when pulling f568b6a on blackknight36:fedora_facts into ba64537 on camptocamp:master.

@DavidS DavidS merged commit 9c3f815 into voxpupuli:master Sep 4, 2017
@DavidS
Copy link
Contributor

DavidS commented Sep 4, 2017

Thank you very much for your contribution!

@blackknight36
Copy link
Contributor Author

No problem. I have noticed a slight issue on modules that only have Fedora 20 listed as a supported OS. For example, rspec will show an error like this.

No facts were found in the FacterDB for: [{:facterversion=>"/\A2\.4\./", :operatingsystem=>"Fedora", :operatingsystemrelease=>"/^20/", :hardwaremodel=>"x86_64"}]

@DavidS
Copy link
Contributor

DavidS commented Sep 5, 2017

@blackknight36 yeah, there seems to be something wrong with the facts you collected. https://github.com/camptocamp/facterdb/pull/50/files#diff-3f46c0bba56680b5787d4de77b8a8d98R16 shows "facterversion": "1.7.6" in the facts/2.4/fedora-20-x86_64.facts file. That's not right.

@blackknight36
Copy link
Contributor Author

@DavidS It looks like the node itself has an older version of facter than the system I'm running tests on.

[root@node ~]# facter --version
1.7.6

Changing the facterversion fact to "2.4.1" does allow tests to run under Fedora 20 as shown below.

ipmi
on fedora-20-x86_64
should contain Package[OpenIPMI] with ensure => "installed"
should contain Package[ipmiutil] with ensure => "installed"
should contain Package[ipmitool] with ensure => "installed"
should contain File[ipmi_conf] that requires Package[OpenIPMI], Package[ipmiutil] and Package[ipmitool]
on centos-7-x86_64
should contain Package[OpenIPMI] with ensure => "installed"
should contain Package[ipmiutil] with ensure => "installed"
should contain Package[ipmitool] with ensure => "installed"
should contain File[ipmi_conf] that requires Package[OpenIPMI], Package[ipmiutil] and Package[ipmitool]

I can submit another PR to fix this if you want.

@DavidS
Copy link
Contributor

DavidS commented Sep 6, 2017

The point of the facterdb is to collect the actual facts from a specific facter version on a specific OS. Just changing the facterversion in the json is not sufficient, since different facter versions have different sets of facts, and would make tests of other modules based on those facts invalid. Even for your own use-case, testing against 2.4 facts is pretty useless, if the code will run against 1.7.6 in production.

@DavidS
Copy link
Contributor

DavidS commented Sep 6, 2017

I've opened #54 to rectify the bigger issue of wrong data.

@blackknight36 blackknight36 deleted the fedora_facts branch September 8, 2017 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants